Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unpin NumPy, Pandas, ASV, Support Python 3.12, Fix Broken Install Link #883

Merged
merged 7 commits into from
Aug 15, 2024
Merged

Unpin NumPy, Pandas, ASV, Support Python 3.12, Fix Broken Install Link #883

merged 7 commits into from
Aug 15, 2024

Conversation

philipc2
Copy link
Member

@philipc2 philipc2 commented Aug 14, 2024 •
edited
Loading

Closes #885, #882, #884

Overview

  • Unpins numpy, pandas and asv packages
  • Adds Python 3.12 to CI workflows
  • Fixes broken install link in README

@philipc2 philipc2 linked an issue Aug 14, 2024 that may be closed by this pull request
Revisit NumPy Pin #882
Closed
@philipc2 philipc2 added the run-benchmark Run ASV benchmark workflow label Aug 14, 2024
@philipc2 philipc2 changed the title Remove NumPy and Pandas Pins DRAFT: Update Enviroments Aug 14, 2024
rajeeja
rajeeja previously approved these changes Aug 14, 2024
View reviewed changes
@philipc2 philipc2 added run-benchmark Run ASV benchmark workflow and removed run-benchmark Run ASV benchmark workflow labels Aug 14, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 14, 2024 •
edited
Loading

ASV Benchmarking

Benchmark Comparison Results

Benchmarks that have improved:

Change Before [1a68daa] After [1c8f7ee] Ratio Benchmark (Parameter)
- 1.20±0.01s 1.03±0.01s 0.86 mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', False)
- 96.0±0.8ms 76.5±0.3ms 0.8 mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', False)
- 402M 354M 0.88 mpas_ocean.Integrate.peakmem_integrate('480km')

Benchmarks that have stayed the same:

Change Before [1a68daa] After [1c8f7ee] Ratio Benchmark (Parameter)
376M 380M 1.01 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
377M 381M 1.01 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
381M 384M 1.01 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
379M 382M 1.01 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
1.40±0.01s 1.33±0s 0.95 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
181±1ms 177±2ms 0.98 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
1.72±0s 1.64±0s 0.96 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
7.83±0.1ms 7.63±0.1ms 0.97 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
1.64±0.01s 1.71±0.02s 1.04 import.Imports.timeraw_import_uxarray
623±8ms 623±7ms 1 mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('120km')
39.1±0.5ms 40.4±0.3ms 1.03 mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('480km')
1.81±0.05ms 1.77±0.01ms 0.98 mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('120km')
476±5μs 474±3μs 1 mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('480km')
1.27±0μs 1.18±0μs 0.93 mpas_ocean.ConstructTreeStructures.time_ball_tree('120km')
299±1ns 302±2ns 1.01 mpas_ocean.ConstructTreeStructures.time_ball_tree('480km')
863±10ns 789±0.9ns 0.91 mpas_ocean.ConstructTreeStructures.time_kd_tree('120km')
285±6ns 276±0.6ns 0.97 mpas_ocean.ConstructTreeStructures.time_kd_tree('480km')
394M 398M 1.01 mpas_ocean.GeoDataFrame.peakmem_to_geodataframe('120km', False)
383M 387M 1.01 mpas_ocean.GeoDataFrame.peakmem_to_geodataframe('120km', True)
356M 362M 1.02 mpas_ocean.GeoDataFrame.peakmem_to_geodataframe('480km', False)
355M 361M 1.02 mpas_ocean.GeoDataFrame.peakmem_to_geodataframe('480km', True)
57.4±1ms 57.8±0.2ms 1.01 mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', True)
5.41±0.1ms 5.42±0.09ms 1 mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', True)
266M 263M 0.99 mpas_ocean.Gradient.peakmem_gradient('120km')
243M 240M 0.99 mpas_ocean.Gradient.peakmem_gradient('480km')
2.64±0.01ms 2.65±0.01ms 1 mpas_ocean.Gradient.time_gradient('120km')
283±1μs 279±2μs 0.99 mpas_ocean.Gradient.time_gradient('480km')
364M 370M 1.02 mpas_ocean.Integrate.peakmem_integrate('120km')
175±0.6ms 177±0.8ms 1.01 mpas_ocean.Integrate.time_integrate('120km')
12.0±0.1ms 12.0±0.1ms 1.01 mpas_ocean.Integrate.time_integrate('480km')
349±3ms 341±2ms 0.98 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'exclude')
345±3ms 344±2ms 0.99 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'include')
348±2ms 344±3ms 0.99 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'split')
22.5±0.2ms 22.2±0.3ms 0.99 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'exclude')
22.4±0.3ms 22.1±0.1ms 0.99 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'include')
22.1±0.06ms 22.0±0.2ms 1 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'split')
55.1±0.2ms 54.8±0.1ms 0.99 mpas_ocean.RemapDownsample.time_inverse_distance_weighted_remapping
44.6±0.2ms 44.3±0.1ms 0.99 mpas_ocean.RemapDownsample.time_nearest_neighbor_remapping
360±0.5ms 361±1ms 1 mpas_ocean.RemapUpsample.time_inverse_distance_weighted_remapping
263±0.6ms 264±0.6ms 1 mpas_ocean.RemapUpsample.time_nearest_neighbor_remapping
239M 238M 1 quad_hexagon.QuadHexagon.peakmem_open_dataset
238M 236M 0.99 quad_hexagon.QuadHexagon.peakmem_open_grid
6.40±0.03ms 6.44±0.1ms 1.01 quad_hexagon.QuadHexagon.time_open_dataset
5.65±0.01ms 5.47±0.05ms 0.97 quad_hexagon.QuadHexagon.time_open_grid

@philipc2
Copy link
Member Author

@rajeeja

Looks like switching to Numpy 2.0 didn't impact performance much. There is however a big improvement with unpinning Pandas for our GeoDataFrame construction.

@philipc2
Copy link
Member Author

Once nschloe/pyfma#17 is merged and released, the errors with pyfma should be fixed.

@philipc2 philipc2 changed the title DRAFT: Update Enviroments DRAFT: Unpin NumPy & Pandas, Support Python 3.12 Aug 14, 2024
@philipc2 philipc2 added dependencies Pull requests that update a dependency file CI Continuous Integration labels Aug 14, 2024
@philipc2 philipc2 linked an issue Aug 14, 2024 that may be closed by this pull request
Add Python 3.12 to CI #885
Closed
@rajeeja rajeeja self-requested a review August 14, 2024 19:27
@rajeeja rajeeja dismissed their stale review August 14, 2024 19:27

fixes for tests needed

@philipc2 philipc2 self-assigned this Aug 14, 2024
@philipc2 philipc2 changed the title DRAFT: Unpin NumPy & Pandas, Support Python 3.12 DRAFT: Unpin NumPy & Pandas, Support Python 3.12, Fix Broken Install Link Aug 14, 2024
@philipc2 philipc2 marked this pull request as ready for review August 14, 2024 19:48
@philipc2 philipc2 changed the title DRAFT: Unpin NumPy & Pandas, Support Python 3.12, Fix Broken Install Link Unpin NumPy & Pandas, Support Python 3.12, Fix Broken Install Link Aug 14, 2024
@philipc2 philipc2 changed the title Unpin NumPy & Pandas, Support Python 3.12, Fix Broken Install Link Unpin NumPy, Pandas, ASV, Support Python 3.12, Fix Broken Install Link Aug 14, 2024
@philipc2 philipc2 requested a review from aaronzedwick August 14, 2024 19:51
@philipc2
Copy link
Member Author

@rajeeja

Should be good for another review.

@philipc2 philipc2 requested a review from erogluorhan August 14, 2024 22:47
rajeeja
rajeeja approved these changes Aug 15, 2024
View reviewed changes
Copy link
Contributor

@rajeeja rajeeja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for removing pins - so important to get this done.
Also closing issue #820

test/test_helpers.py Show resolved Hide resolved
@philipc2 philipc2 merged commit 6deef0a into main Aug 15, 2024
20 checks passed
@erogluorhan erogluorhan deleted the philipc2/numpy-2 branch December 2, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rajeeja rajeeja rajeeja approved these changes

@aaronzedwick aaronzedwick Awaiting requested review from aaronzedwick

@erogluorhan erogluorhan Awaiting requested review from erogluorhan

Assignees

@philipc2 philipc2

Labels
CI Continuous Integration dependencies Pull requests that update a dependency file run-benchmark Run ASV benchmark workflow
Projects
None yet
Milestone
Package Infrastructure & CI/CD
Development

Successfully merging this pull request may close these issues.

Add Python 3.12 to CI Revisit NumPy Pin
2 participants
@philipc2 @rajeeja